Correct route and dynamic param extraction for Expo Router (#6157)#6197
Open
alwx wants to merge 3 commits into
Open
Correct route and dynamic param extraction for Expo Router (#6157)#6197alwx wants to merge 3 commits into
alwx wants to merge 3 commits into
Conversation
Contributor
Author
|
@cursor review this please |
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 31c8c4f. Configure here.
antonis
reviewed
May 21, 2026
antonis
reviewed
May 21, 2026
Contributor
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
Other than the API change issue LGTM!
… API report Switch the JSDoc on `_setRouteOverrideProvider` from a plain "Internal API" sentence to the api-extractor `@internal` release tag and regenerate `etc/sentry-react-native.api.md` so the new field is recorded with the correct tag. Without this, the `api-report:check` step fails because the public API signature changed but the report wasn't updated. Per review feedback on #6197.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4f09db5. Configure here.
| latestNavigationSpan.setAttributes({ | ||
| 'route.name': routeName, | ||
| 'route.key': route.key, | ||
| ...(sendDefaultPii ? extractDynamicRouteParams(routeName, route.params) : undefined), |
There was a problem hiding this comment.
Same-key state drops override
Medium Severity
When a navigation state event fires with the same route.key as before, latestRoute is reset to the raw React Navigation route object. After an Expo Router override, that drops the templated route.name stored on latestRoute, so the next span’s previous_route.name and navigation breadcrumbs can revert to file names like index instead of paths like /profile/[id].
Reviewed by Cursor Bugbot for commit 4f09db5. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+144
to
+151
| return undefined; | ||
| } | ||
| if (!info) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const templatedPath = buildExpoRouterTemplatedPath(info.segments); | ||
| return { |
There was a problem hiding this comment.
Bug: If getRouteInfo() returns an object without a segments property, all navigation routes will be incorrectly named '/' because buildExpoRouterTemplatedPath is called with undefined.
Severity: MEDIUM
Suggested Fix
In buildExpoRouterRouteOverride, add a guard to check if info.segments is defined and has a length greater than zero before calling buildExpoRouterTemplatedPath. If info.segments is missing or empty, buildExpoRouterRouteOverride should return undefined to prevent an incorrect override from being generated.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/js/tracing/expoRouterIntegration.ts#L144-L151
Potential issue: When Expo Router's `getRouteInfo()` returns an object that exists but
lacks the `segments` property, `buildExpoRouterTemplatedPath(info.segments)` is called
with `undefined`. This function then returns a default path of `'/'`. Because `'/'` is a
truthy value, `buildExpoRouterRouteOverride` creates a valid `RouteOverride` which is
then applied in `updateLatestNavigationSpanWithCurrentRoute`. This results in all
subsequent navigation events being incorrectly labeled with the route name `'/'`,
regardless of the actual route being navigated to. The `ExpoRouterUrlObject` interface
explicitly marks `segments` as optional, confirming this is a possible scenario.
Did we get this right? 👍 / 👎 to inform future reviews.
📲 Install BuildsAndroid
|
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3817909+dirty | 1183.90 ms | 1187.50 ms | 3.60 ms |
| 44c8b3f+dirty | 3823.85 ms | 1207.66 ms | -2616.19 ms |
| b0d3373+dirty | 3831.75 ms | 1227.29 ms | -2604.46 ms |
| 7d6fd3a+dirty | 1223.29 ms | 1229.57 ms | 6.28 ms |
| 6176a94+dirty | 3836.50 ms | 1217.64 ms | -2618.86 ms |
| 5c1e987+dirty | 1204.30 ms | 1222.15 ms | 17.85 ms |
| 3d377b5+dirty | 1218.48 ms | 1219.51 ms | 1.03 ms |
| 7d8c8bd+dirty | 3837.24 ms | 1215.51 ms | -2621.73 ms |
| 4966363+dirty | 3854.04 ms | 1231.55 ms | -2622.50 ms |
| 5fe1c6c+dirty | 1220.79 ms | 1217.63 ms | -3.16 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3817909+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 44c8b3f+dirty | 5.15 MiB | 6.66 MiB | 1.51 MiB |
| b0d3373+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 7d6fd3a+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 6176a94+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 5c1e987+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 3d377b5+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 7d8c8bd+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 4966363+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 5fe1c6c+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3817909+dirty | 1210.76 ms | 1215.64 ms | 4.89 ms |
| 44c8b3f+dirty | 3849.24 ms | 1209.94 ms | -2639.31 ms |
| b0d3373+dirty | 3842.49 ms | 1218.49 ms | -2624.00 ms |
| 7d6fd3a+dirty | 1210.89 ms | 1217.63 ms | 6.74 ms |
| 6176a94+dirty | 3854.15 ms | 1221.77 ms | -2632.38 ms |
| 5c1e987+dirty | 1208.43 ms | 1220.72 ms | 12.29 ms |
| 3d377b5+dirty | 1201.55 ms | 1201.80 ms | 0.25 ms |
| 7d8c8bd+dirty | 3847.98 ms | 1230.77 ms | -2617.21 ms |
| 4966363+dirty | 3863.07 ms | 1227.19 ms | -2635.88 ms |
| 5fe1c6c+dirty | 1201.36 ms | 1209.15 ms | 7.78 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3817909+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 44c8b3f+dirty | 5.15 MiB | 6.66 MiB | 1.51 MiB |
| b0d3373+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 7d6fd3a+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 6176a94+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 5c1e987+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 3d377b5+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 7d8c8bd+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 4966363+dirty | 5.15 MiB | 6.68 MiB | 1.53 MiB |
| 5fe1c6c+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
Contributor
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b9bebee+dirty | 500.50 ms | 536.42 ms | 35.92 ms |
| 71abba0+dirty | 411.04 ms | 453.67 ms | 42.63 ms |
| 5569641+dirty | 465.92 ms | 532.22 ms | 66.30 ms |
| 3b6e9f9+dirty | 442.39 ms | 486.44 ms | 44.05 ms |
| 6177334+dirty | 404.80 ms | 456.74 ms | 51.94 ms |
| ef27341+dirty | 519.02 ms | 553.42 ms | 34.40 ms |
| 23598c3+dirty | 371.92 ms | 420.65 ms | 48.74 ms |
| a0d8cf8+dirty | 533.71 ms | 564.25 ms | 30.54 ms |
| 7ac3378+dirty | 410.67 ms | 442.60 ms | 31.92 ms |
| c004dae+dirty | 404.60 ms | 430.67 ms | 26.07 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b9bebee+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 71abba0+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| 5569641+dirty | 48.30 MiB | 53.48 MiB | 5.18 MiB |
| 3b6e9f9+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 6177334+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| ef27341+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 23598c3+dirty | 43.94 MiB | 49.02 MiB | 5.08 MiB |
| a0d8cf8+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| 7ac3378+dirty | 43.94 MiB | 48.99 MiB | 5.05 MiB |
| c004dae+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
Contributor
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7d8c8bd+dirty | 417.45 ms | 462.10 ms | 44.65 ms |
| 71abba0+dirty | 496.54 ms | 525.16 ms | 28.63 ms |
| b9bebee+dirty | 438.86 ms | 452.21 ms | 13.35 ms |
| 23598c3+dirty | 414.12 ms | 426.24 ms | 12.12 ms |
| a5d243c+dirty | 424.52 ms | 485.18 ms | 60.66 ms |
| 0b5120f+dirty | 503.22 ms | 538.60 ms | 35.38 ms |
| 6176a94+dirty | 410.90 ms | 452.20 ms | 41.31 ms |
| 7ac3378+dirty | 404.78 ms | 439.84 ms | 35.06 ms |
| 6177334+dirty | 408.16 ms | 441.14 ms | 32.98 ms |
| 5a21b51+dirty | 471.42 ms | 524.22 ms | 52.80 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7d8c8bd+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 71abba0+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| b9bebee+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 23598c3+dirty | 43.75 MiB | 48.16 MiB | 4.41 MiB |
| a5d243c+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 0b5120f+dirty | 48.30 MiB | 53.58 MiB | 5.28 MiB |
| 6176a94+dirty | 48.30 MiB | 53.54 MiB | 5.24 MiB |
| 7ac3378+dirty | 43.75 MiB | 48.13 MiB | 4.37 MiB |
| 6177334+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 5a21b51+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Fixes #6157
The idea is simple: when Expo Router is active, we can get the route from Expo Router's own state rather than from getCurrentRoute().name. That allows for this information about route and params to be more precise and detailed compared to the filename-based approach of React Navigation.
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps